Skip to content

Docker: Chain egress proxy through upstream HTTP(S)_PROXY - #2142

Open
SohamKukreti wants to merge 1 commit into
developfrom
fix/egress-proxy-upstream-chaining
Open

Docker: Chain egress proxy through upstream HTTP(S)_PROXY#2142
SohamKukreti wants to merge 1 commit into
developfrom
fix/egress-proxy-upstream-chaining

Conversation

@SohamKukreti

Copy link
Copy Markdown
Collaborator

Summary

Since 0.9.0, the Docker server's egress pinning proxy dials target IPs directly (asyncio.open_connection), ignoring HTTP_PROXY/HTTPS_PROXY. On hosts that can only reach the internet through a corporate proxy, every crawl fails with Page.goto: net::ERR_TIMED_OUT — this worked in 0.8.x. Reported in discussion #2041.

This PR teaches the pinning proxy to chain through the upstream proxy without weakening the SSRF/rebinding guarantees: validation (resolve_and_pin) still runs locally before the upstream is ever contacted, and the upstream is asked to CONNECT <pinned-ip>:<port> — never a hostname — so it performs no DNS resolution of its own and the pin holds.

Behavior:

  • Reads standard HTTP_PROXY / HTTPS_PROXY / NO_PROXY (both cases; scheme-aware selection with fallback); CRAWL4AI_UPSTREAM_PROXY overrides them.
  • NO_PROXY supports domain-suffix (.corp.lan) and IP/CIDR (192.168.0.0/16) entries — internal targets should be listed there to dial direct.
  • Basic auth via http://user:pass@proxy:port (percent-decoded, UTF-8) sent preemptively; a 407/non-200 from the upstream fails fast with the existing opaque 403 and a server-side log line.
  • Plain-HTTP requests are re-issued to the upstream in absolute form against the pinned IP with Connection: close forced, so a reused client connection cannot smuggle unvalidated requests past the pin.
  • With no proxy env vars set, the dial path is byte-identical to current behavior — zero change for existing deployments.

Known limitations (documented in the README):

  • Proxies that refuse CONNECT-to-an-IP (hostname/domain ACLs) and containers with no DNS at all are not supported yet (a future opt-in hostname mode could cover them).
  • NTLM/Kerberos/SPNEGO proxies are not supported — front them with a local translator (e.g. cntlm, px) and point CRAWL4AI_UPSTREAM_PROXY at it.

List of files changed and why

  • deploy/docker/egress_proxy.py — upstream detection (upstream_proxy(), _no_proxy_match(), _use_upstream()), the chained _dial() (CONNECT-to-pinned-IP, auth, header drain so upstream headers never leak into the tunnel), absolute-form + Connection: close for chained plain HTTP, startup/refusal log lines.
  • deploy/docker/tests/test_security_egress_proxy.py — 3 new tests covering the security contract: chained CONNECT carries only the pinned IP and blocked targets produce zero upstream traffic; plain-HTTP absolute-form + anti-smuggling; env parsing (precedence, scheme selection, credentials, NO_PROXY routing). Plus an autouse fixture clearing proxy env vars so the suite is deterministic on dev machines behind proxies.
  • deploy/docker/README.md — "Behind a corporate proxy" note in the Run the Container section (env vars, auth support, limitations).

How Has This Been Tested?

  • Full Docker security suite: 319 passed, 1 xfailed (includes the 0.9 hardening contract tests, all unchanged).
  • Library regression suite (tests/regression/, non-network): 293 passed; the single failure (test_cosine_basic, missing optional transformers) was verified pre-existing by re-running on the unmodified tree.
  • Adversarial pass (32 targeted tests, real sockets, no mocking): env parsing edge cases (malformed/whitespace URLs, non-latin-1 passwords), NO_PROXY matching (anchored suffixes, CIDR, the exact list from [Feature Request]: Support for docker proxy #2041), dead/407/garbage upstreams (fast opaque 403, no hang), IPv6 pin bracketing, 8 concurrent tunnels with no cross-talk, and the smuggling regression.
  • End-to-end: real Chromium → pinning proxy → a real forwarding "corporate" proxy → origin server, reproducing the [Feature Request]: Support for docker proxy #2041 topology — page + subresources each arrive as separate validated pinned-IP requests.
  • Proxy-auth probe: preemptive Basic accepted first-try by an auth-requiring proxy; wrong/missing creds and NTLM-only proxies fail fast (single attempt, opaque 403, warning logged).

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added/updated unit tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Dial via the corporate proxy by CONNECT-to-the-pinned-IP when proxy env vars
are set, restoring crawls on proxy-only hosts (discussion #2041) without
weakening SSRF/rebinding guarantees.
@fchinch

fchinch commented Aug 18, 2026

Copy link
Copy Markdown

Thanks for this — the chaining works exactly as described for the standard
"container can only reach the internet through a corporate proxy" case, and the
CONNECT <pinned-ip> design keeps the rebinding guarantee intact. Nice.

I wanted to flag a related deployment shape that this doesn't cover yet, in case
it's useful input for the opt-in hostname mode you mention under known limitations.

Our setup: the proxy isn't an egress proxy — it's the entry point of a reverse
tunnel. An agent runs inside the customer's private network and dials out to our
hub; the hub exposes an HTTP CONNECT endpoint. When the browser CONNECTs, the hub
forwards the request over that tunnel and the agent performs the DNS resolution
and the connection from inside the customer's network
. That's the whole point:
it lets us crawl sites that only exist there.

Why the PR doesn't reach that case, all three by design rather than oversight:

  1. resolve_and_pin runs locally first, and an internal hostname doesn't resolve
    from our side at all.
  2. Even when it does resolve, the answer is RFC1918 and is rejected before the
    upstream is considered.
  3. The upstream receives CONNECT <pinned-ip>:<port> — but the far side needs the
    hostname, because resolution is supposed to happen over there. An IP we
    resolved locally is either wrong or meaningless in the target network.

NO_PROXY points the other way too: it treats internal targets as things to dial
direct, whereas for us internal targets are precisely what must traverse the proxy.

What would unblock it is the opt-in hostname mode you already floated: when an
operator explicitly configures an upstream and opts in, send CONNECT <hostname>
and let the upstream resolve, skipping the local pin for that path. That is a
deliberate trust decision — you're trusting a proxy you configured — so gating it
behind an explicit env var (rather than inferring it from HTTP_PROXY) seems right.

Happy to test a branch against our stack if you build one; we have a reproducible
environment with the tunnel and a private-network target, and can report back with
logs.

fchinch added a commit to fchinch/crawl4ai that referenced this pull request Aug 18, 2026
Builds on the upstream-chaining work in unclecode#2142, which asks the upstream to
CONNECT to an already-pinned IP so the rebinding guarantee holds. That is the
right default, but it cannot serve an upstream that fronts a network whose
names do not resolve on our side, or one that enforces hostname ACLs and so
refuses CONNECT-to-an-IP — both listed there as known limitations.

CRAWL4AI_UPSTREAM_PROXY_DNS_SUFFIXES names the suffixes an operator wants the
upstream to resolve. Matching hostnames skip resolve_and_pin and are sent to
the upstream verbatim; the upstream then owns where the connection lands.

Two layers had to honour it. The pinning proxy is the obvious one. The entry
point check in validate_url_destination runs before the browser starts, and
rejected delegated names there — so the request never reached the proxy and
the suffix list had no effect. Verified against a live tunnel: without the
second change the crawl still returned "URL blocked (SSRF protection)", and
the only way through was CRAWL4AI_ALLOW_INTERNAL_URLS, which is exactly the
blunt instrument this is meant to avoid.

Deliberately an allowlist rather than a boolean, so the pin is given up only
for names an operator named, never wholesale:

  - unset (the default) leaves every path byte-identical to today;
  - a name outside the list keeps resolve-and-pin;
  - an IP literal never qualifies, because there is no name to delegate — this
    stops a suffix entry from reaching 169.254.169.254;
  - NO_PROXY still exempts hosts, matched by name since no IP is learned.

Eight tests cover that contract, four per layer: the allowlisted name reaches
the upstream unresolved, a name outside the list still arrives pinned, an IP
literal is refused with zero upstream traffic even with a wildcard suffix, and
an unset variable changes nothing.
fchinch added a commit to fchinch/crawl4ai that referenced this pull request Aug 18, 2026
Builds on the upstream-chaining work in unclecode#2142, which asks the upstream to
CONNECT to an already-pinned IP so the rebinding guarantee holds. That is the
right default, but it cannot serve an upstream that fronts a network whose
names do not resolve on our side, or one that enforces hostname ACLs and so
refuses CONNECT-to-an-IP — both listed there as known limitations.

CRAWL4AI_UPSTREAM_PROXY_DNS_SUFFIXES names the suffixes an operator wants the
upstream to resolve. Matching hostnames skip resolve_and_pin and are sent to
the upstream verbatim; the upstream then owns where the connection lands.

Two layers had to honour it. The pinning proxy is the obvious one. The entry
point check in validate_url_destination runs before the browser starts, and
rejected delegated names there — so the request never reached the proxy and
the suffix list had no effect. Verified against a live tunnel: without the
second change the crawl still returned "URL blocked (SSRF protection)", and
the only way through was CRAWL4AI_ALLOW_INTERNAL_URLS, which is exactly the
blunt instrument this is meant to avoid.

Deliberately an allowlist rather than a boolean, so the pin is given up only
for names an operator named, never wholesale:

  - unset (the default) leaves every path byte-identical to today;
  - a name outside the list keeps resolve-and-pin;
  - an IP literal never qualifies, because there is no name to delegate — this
    stops a suffix entry from reaching 169.254.169.254;
  - NO_PROXY still exempts hosts, matched by name since no IP is learned.

Eight tests cover that contract, four per layer: the allowlisted name reaches
the upstream unresolved, a name outside the list still arrives pinned, an IP
literal is refused with zero upstream traffic even with a wildcard suffix, and
an unset variable changes nothing.
fchinch added a commit to fchinch/crawl4ai that referenced this pull request Aug 18, 2026
Builds on the upstream-chaining work in unclecode#2142, which asks the upstream to
CONNECT to an already-pinned IP so the rebinding guarantee holds. That is the
right default, but it cannot serve an upstream that fronts a network whose
names do not resolve on our side, or one that enforces hostname ACLs and so
refuses CONNECT-to-an-IP — both listed there as known limitations.

CRAWL4AI_UPSTREAM_PROXY_DNS_SUFFIXES names the suffixes an operator wants the
upstream to resolve. Matching hostnames skip resolve_and_pin and are sent to
the upstream verbatim; the upstream then owns where the connection lands.

Two layers had to honour it. The pinning proxy is the obvious one. The entry
point check in validate_url_destination runs before the browser starts, and
rejected delegated names there — so the request never reached the proxy and
the suffix list had no effect. Verified against a live tunnel: without the
second change the crawl still returned "URL blocked (SSRF protection)", and
the only way through was CRAWL4AI_ALLOW_INTERNAL_URLS, which is exactly the
blunt instrument this is meant to avoid.

Deliberately an allowlist rather than a boolean, so the pin is given up only
for names an operator named, never wholesale:

  - unset (the default) leaves every path byte-identical to today;
  - a name outside the list keeps resolve-and-pin;
  - an IP literal never qualifies, because there is no name to delegate — this
    stops a suffix entry from reaching 169.254.169.254;
  - NO_PROXY still exempts hosts, matched by name since no IP is learned.

Eight tests cover that contract, four per layer: the allowlisted name reaches
the upstream unresolved, a name outside the list still arrives pinned, an IP
literal is refused with zero upstream traffic even with a wildcard suffix, and
an unset variable changes nothing.
fchinch added a commit to fchinch/crawl4ai that referenced this pull request Aug 18, 2026
Builds on the upstream-chaining work in unclecode#2142, which asks the upstream to
CONNECT to an already-pinned IP so the rebinding guarantee holds. That is the
right default, but it cannot serve an upstream that fronts a network whose
names do not resolve on our side, or one that enforces hostname ACLs and so
refuses CONNECT-to-an-IP — both listed there as known limitations.

CRAWL4AI_UPSTREAM_PROXY_DNS_SUFFIXES names the suffixes an operator wants the
upstream to resolve. Matching hostnames skip resolve_and_pin and are sent to
the upstream verbatim; the upstream then owns where the connection lands.

Two layers had to honour it. The pinning proxy is the obvious one. The entry
point check in validate_url_destination runs before the browser starts, and
rejected delegated names there — so the request never reached the proxy and
the suffix list had no effect. Verified against a live tunnel: without the
second change the crawl still returned "URL blocked (SSRF protection)", and
the only way through was CRAWL4AI_ALLOW_INTERNAL_URLS, which is exactly the
blunt instrument this is meant to avoid.

Deliberately an allowlist rather than a boolean, so the pin is given up only
for names an operator named, never wholesale:

  - unset (the default) leaves every path byte-identical to today;
  - a name outside the list keeps resolve-and-pin;
  - an IP literal never qualifies, because there is no name to delegate — this
    stops a suffix entry from reaching 169.254.169.254;
  - NO_PROXY still exempts hosts, matched by name since no IP is learned.

Eight tests cover that contract, four per layer: the allowlisted name reaches
the upstream unresolved, a name outside the list still arrives pinned, an IP
literal is refused with zero upstream traffic even with a wildcard suffix, and
an unset variable changes nothing.
fchinch added a commit to fchinch/crawl4ai that referenced this pull request Aug 18, 2026
Builds on the upstream-chaining work in unclecode#2142, which asks the upstream to
CONNECT to an already-pinned IP so the rebinding guarantee holds. That is the
right default, but it cannot serve an upstream that fronts a network whose
names do not resolve on our side, or one that enforces hostname ACLs and so
refuses CONNECT-to-an-IP — both listed there as known limitations.

CRAWL4AI_UPSTREAM_PROXY_DNS_SUFFIXES names the suffixes an operator wants the
upstream to resolve. Matching hostnames skip resolve_and_pin and are sent to
the upstream unresolved; the upstream then owns where the connection lands.

Two layers had to honour it. The pinning proxy is the obvious one. The entry
point check in validate_url_destination runs before the browser starts, and
rejected delegated names there — so the request never reached the proxy and
the suffix list had no effect. Verified against a live tunnel: without the
second change the crawl still returned "URL blocked (SSRF protection)", and
the only way through was CRAWL4AI_ALLOW_INTERNAL_URLS, which is exactly the
blunt instrument this is meant to avoid. Both layers now share one set of
helpers rather than parsing the list twice, so they cannot drift into
disagreeing about which names are delegated.

Deliberately an allowlist rather than a boolean, so the pin is given up only
for names an operator named, never wholesale:

  - unset (the default) leaves every path byte-identical to today;
  - a name outside the list keeps resolve-and-pin;
  - an IP literal never qualifies, because there is no name to delegate — this
    stops a suffix entry from reaching 169.254.169.254;
  - a wildcard entry is refused with a warning rather than honoured, since
    delegating every name would turn any caller-supplied URL into a lookup
    performed by the upstream;
  - delegated names are reachable on ports 80 and 443 only, configurable via
    CRAWL4AI_UPSTREAM_PROXY_DNS_PORTS. The pinned path leaves the port open but
    requires a global address; passthrough gives up exactly that check, so
    without a port policy a listed suffix could reach :9200 or :5432 rather
    than a web server;
  - NO_PROXY still exempts hosts, matched by name since no IP is learned.

Names are compared in normalised form — case-folded, root dot stripped,
IDNA-encoded, length checked before and after encoding — and that normalised
name is what is sent to the upstream, so the name the allowlist authorised is
the name that goes on the wire rather than the caller's original spelling.

Fifty-four tests cover the contract, including what the upstream actually
receives: a listed name reaches it unresolved while a name outside the list
still arrives pinned, three spellings of one name all arrive as the single
authorised form, an IP literal is refused with zero upstream traffic, ports
outside the allowed set fall back to the pin, a set-but-unusable port list
closes rather than opens, and an unset variable changes nothing. The delegated
plain-HTTP path carries upstream auth and Connection: close, so a reused client
connection cannot smuggle a second, unchecked request upstream.
@fchinch

fchinch commented Aug 18, 2026

Copy link
Copy Markdown

@SohamKukreti @unclecode I added this PR:
#2152

fchinch added a commit to fchinch/crawl4ai that referenced this pull request Aug 19, 2026
Builds on the upstream-chaining work in unclecode#2142, which asks the upstream to
CONNECT to an already-pinned IP so the rebinding guarantee holds. That is the
right default, but it cannot serve an upstream that fronts a network whose
names do not resolve on our side, or one that enforces hostname ACLs and so
refuses CONNECT-to-an-IP — both listed there as known limitations.

CRAWL4AI_UPSTREAM_PROXY_DNS_SUFFIXES names the suffixes an operator wants the
upstream to resolve. Matching hostnames skip resolve_and_pin and are sent to
the upstream unresolved; the upstream then owns where the connection lands.

Two layers had to honour it. The pinning proxy is the obvious one. The entry
point check in validate_url_destination runs before the browser starts, and
rejected delegated names there — so the request never reached the proxy and
the suffix list had no effect. Verified against a live tunnel: without the
second change the crawl still returned "URL blocked (SSRF protection)", and
the only way through was CRAWL4AI_ALLOW_INTERNAL_URLS, which is exactly the
blunt instrument this is meant to avoid. Both layers now share one set of
helpers rather than parsing the list twice, so they cannot drift into
disagreeing about which names are delegated.

Deliberately an allowlist rather than a boolean, so the pin is given up only
for names an operator named, never wholesale:

  - unset (the default) leaves every path byte-identical to today;
  - a name outside the list keeps resolve-and-pin;
  - an IP literal never qualifies, because there is no name to delegate — this
    stops a suffix entry from reaching 169.254.169.254;
  - a wildcard entry is refused with a warning rather than honoured, since
    delegating every name would turn any caller-supplied URL into a lookup
    performed by the upstream;
  - delegated names are reachable on ports 80 and 443 only, configurable via
    CRAWL4AI_UPSTREAM_PROXY_DNS_PORTS. The pinned path leaves the port open but
    requires a global address; passthrough gives up exactly that check, so
    without a port policy a listed suffix could reach :9200 or :5432 rather
    than a web server;
  - NO_PROXY still exempts hosts, matched by name since no IP is learned.

Names are compared in normalised form — case-folded, root dot stripped,
IDNA-encoded, length checked before and after encoding — and that normalised
name is what is sent to the upstream, so the name the allowlist authorised is
the name that goes on the wire rather than the caller's original spelling.

Fifty-four tests cover the contract, including what the upstream actually
receives: a listed name reaches it unresolved while a name outside the list
still arrives pinned, three spellings of one name all arrive as the single
authorised form, an IP literal is refused with zero upstream traffic, ports
outside the allowed set fall back to the pin, a set-but-unusable port list
closes rather than opens, and an unset variable changes nothing. The delegated
plain-HTTP path carries upstream auth and Connection: close, so a reused client
connection cannot smuggle a second, unchecked request upstream.

CRAWL4AI_UPSTREAM_PROXY_ONLY_SUFFIXES narrows which destinations are chained at
all. Chaining is otherwise all-or-nothing — with an upstream set every target
goes through it, and NO_PROXY only subtracts exceptions from that — so a single
deployment cannot serve an internal-only upstream and direct public egress at
the same time, because the public set cannot be enumerated in NO_PROXY. Sending
public crawls through the upstream anyway is not just wasteful: an upstream that
authorises per destination refuses them, and the fetch is attributed to its
network rather than ours. Listing suffixes here inverts the rule for those names
— they are chained, everything else dials direct.

It composes with CRAWL4AI_UPSTREAM_PROXY_DNS_SUFFIXES rather than overriding it:
a name may be handed over unresolved only if it is also allowed to use the
upstream, so neither list can widen what the other permits. Unset, the default,
leaves every path byte-identical to today. The change only ever adds a reason to
skip the upstream, so a target that is not chained is resolved and pinned exactly
as before — the pin covers strictly more traffic than it did, never less.

Six further tests cover it: an unset variable changes nothing, a listed suffix is
still chained as the pinned IP, an unlisted host produces zero upstream traffic,
a substring such as notcorp.example does not match .corp.example, delegation is
refused when the DNS list allows a name the upstream allowlist does not, and
wildcard or malformed entries are ignored with the list then behaving as absent.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fchinch added a commit to fchinch/crawl4ai that referenced this pull request Aug 19, 2026
Builds on the upstream-chaining work in unclecode#2142, which asks the upstream to
CONNECT to an already-pinned IP so the rebinding guarantee holds. That is the
right default, but it cannot serve an upstream that fronts a network whose
names do not resolve on our side, or one that enforces hostname ACLs and so
refuses CONNECT-to-an-IP — both listed there as known limitations.

CRAWL4AI_UPSTREAM_PROXY_DNS_SUFFIXES names the suffixes an operator wants the
upstream to resolve. Matching hostnames skip resolve_and_pin and are sent to
the upstream unresolved; the upstream then owns where the connection lands.

Two layers had to honour it. The pinning proxy is the obvious one. The entry
point check in validate_url_destination runs before the browser starts, and
rejected delegated names there — so the request never reached the proxy and
the suffix list had no effect. Verified against a live tunnel: without the
second change the crawl still returned "URL blocked (SSRF protection)", and
the only way through was CRAWL4AI_ALLOW_INTERNAL_URLS, which is exactly the
blunt instrument this is meant to avoid. Both layers now share one set of
helpers rather than parsing the list twice, so they cannot drift into
disagreeing about which names are delegated.

Deliberately an allowlist rather than a boolean, so the pin is given up only
for names an operator named, never wholesale:

  - unset (the default) leaves every path byte-identical to today;
  - a name outside the list keeps resolve-and-pin;
  - an IP literal never qualifies, because there is no name to delegate — this
    stops a suffix entry from reaching 169.254.169.254;
  - a wildcard entry is refused with a warning rather than honoured, since
    delegating every name would turn any caller-supplied URL into a lookup
    performed by the upstream;
  - delegated names are reachable on ports 80 and 443 only, configurable via
    CRAWL4AI_UPSTREAM_PROXY_DNS_PORTS. The pinned path leaves the port open but
    requires a global address; passthrough gives up exactly that check, so
    without a port policy a listed suffix could reach :9200 or :5432 rather
    than a web server;
  - NO_PROXY still exempts hosts, matched by name since no IP is learned.

Names are compared in normalised form — case-folded, root dot stripped,
IDNA-encoded, length checked before and after encoding — and that normalised
name is what is sent to the upstream, so the name the allowlist authorised is
the name that goes on the wire rather than the caller's original spelling.

Fifty-four tests cover the contract, including what the upstream actually
receives: a listed name reaches it unresolved while a name outside the list
still arrives pinned, three spellings of one name all arrive as the single
authorised form, an IP literal is refused with zero upstream traffic, ports
outside the allowed set fall back to the pin, a set-but-unusable port list
closes rather than opens, and an unset variable changes nothing. The delegated
plain-HTTP path carries upstream auth and Connection: close, so a reused client
connection cannot smuggle a second, unchecked request upstream.

CRAWL4AI_UPSTREAM_PROXY_ONLY_SUFFIXES narrows which destinations are chained at
all. Chaining is otherwise all-or-nothing — with an upstream set every target
goes through it, and NO_PROXY only subtracts exceptions from that — so a single
deployment cannot serve an internal-only upstream and direct public egress at
the same time, because the public set cannot be enumerated in NO_PROXY. Sending
public crawls through the upstream anyway is not just wasteful: an upstream that
authorises per destination refuses them, and the fetch is attributed to its
network rather than ours. Listing suffixes here inverts the rule for those names
— they are chained, everything else dials direct.

It composes with CRAWL4AI_UPSTREAM_PROXY_DNS_SUFFIXES rather than overriding it:
a name may be handed over unresolved only if it is also allowed to use the
upstream, so neither list can widen what the other permits. Unset, the default,
leaves every path byte-identical to today. The change only ever adds a reason to
skip the upstream, so a target that is not chained is resolved and pinned exactly
as before — the pin covers strictly more traffic than it did, never less.

Six further tests cover it: an unset variable changes nothing, a listed suffix is
still chained as the pinned IP, an unlisted host produces zero upstream traffic,
a substring such as notcorp.example does not match .corp.example, delegation is
refused when the DNS list allows a name the upstream allowlist does not, and
wildcard or malformed entries are ignored with the list then behaving as absent.

@ntohidi ntohidi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work — the security contract holds. I checked the part that matters: resolve_and_pin runs before the upstream is ever contacted, the upstream only ever gets CONNECT <ip>:<port> (never a name, so it has nothing to resolve), its reply headers are drained so they can't leak into the tunnel, and with no proxy env set the dial path is unchanged. Ran the suite locally — the 3 new tests pass.

Two small things first, both in upstream_proxy() (deploy/docker/egress_proxy.py:45). Neither breaks anyone who works today, but both fail silently, which is the same mystery timeout that got #2041 opened.

1. A junk value hides a good one. _env takes the first var that is set, then we bail if it won't parse — so a valid fallback never gets a look:

HTTPS_PROXY="   "      + HTTP_PROXY="http://good:3128"  ->  None
HTTPS_PROXY="http://"  + HTTP_PROXY="http://good:3128"  ->  None

Result: proxy silently off, every crawl times out. Fix: keep the first candidate that parses, not the first that exists.

2. An https:// proxy URL is mis-dialed.

HTTPS_PROXY=https://proxy.corp  ->  ('proxy.corp', 80, None)

Port should be 443, and we open a plain socket rather than TLS. A logger.warning (or an outright refusal) would be enough — right now the operator gets a timeout and no clue.

Two nits, take them or leave them:

  • NO_PROXY=site.example:80 never matches — the port breaks the suffix test, and real NO_PROXY lists often carry ports.
  • The description says a 407 "fails fast with the existing opaque 403". That's the CONNECT path only; on plain HTTP the upstream 407 is spliced straight back to Chromium. Not a security issue, just worth correcting in the text.

Happy to merge once 1 and 2 are in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants